Skip to content

Monalisa's Bank Accounts#39

Open
MonalisaC wants to merge 23 commits intoAda-C7:masterfrom
MonalisaC:master
Open

Monalisa's Bank Accounts#39
MonalisaC wants to merge 23 commits intoAda-C7:masterfrom
MonalisaC:master

Conversation

@MonalisaC
Copy link

@MonalisaC MonalisaC commented Feb 27, 2017

Bank Account

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
Why is it useful to put classes inside modules? What do you think? for namespace which is abstract container or environment created to hold a logical grouping of unique identifiers or symbols (i.e. names)
What is accomplished with raise ArgumentError? What do you think it's doing? It raises an exception and it is raised when the arguments are wrong 
|

| Why do you think we made the .all & .find methods class methods? Why not instance methods? | It exist in the object that defined them (the class) and nowhere else and if we do instance method it'll give nomethod error |
| Why does it make sense to use inheritance for the different types of accounts? | lowers down the hierarchy |
| Did the presence of automated tests change the way you thought about the problem? How? | yes it helps to define the problem and increases its scope for further changes.|

@MonalisaC
respond to :id, :balance
26ef249 
q
@MonalisaC
balance must not be negative
a71e5b5
@MonalisaC
Withdrawl test passed
4f659dd
@MonalisaC
insufficient fund
59ce982
@MonalisaC
amount must be >= 0
2da5b89
@MonalisaC
method deposit increase balance and puts modified balance.
9ba6c8d
@MonalisaC
require a positive deposit amount.
15351eb
@MonalisaC
owner initialize.
4c58457
@MonalisaC
add owner method
5673f40
@MonalisaC
Changed 'xdescribe to 'describe'.
92ef020
@MonalisaC
Test: What is Account.all
d522082
@MonalisaC
id test using string
3a48e92
@MonalisaC
Test describe Account.find
d3b2bab
@MonalisaC
Testing for class method self.all return owner instances.
541de3f
@MonalisaC
Added self.find class method in owner class and tested it to return a…
8b9412b 
…n instance of owner.
@MonalisaC
Create two new classes CheckingAccount < Account and SavingsAccount <…
03f0647 
… Account under lib/directory.
@MonalisaC
Convert the values from cents to dollar in self.all (lib/account.rb).
2e5ad61
@MonalisaC
Describe savings account and withdraw method.
3eb43e8
@MonalisaC
Describe add_interest method in savings a/c.
9154766
@MonalisaC
Describe methode withdraw in checking a/c.
9ea4f9d
@MonalisaC
intialized checks in class CheckingAccount
2d0bc30
@MonalisaC
Tested withdraw using checks in checking a/c.
e6da934
@MonalisaC
Fixed balance must equal value in dollar.
454c45d
@kariabancroft
Copy link

kariabancroft commented Feb 27, 2017

Bank Account

What We're Looking For

Feature Feedback
Wave 1
All provided tests pass Yes
Using the appropriate attr_ for instance variables Yes
Wave 2
All stubbed tests are implemented fully and pass Not quite. The assertions for the all method should be split into separate it blocks to make it more clear which assertions go with which specific situations. Your find tests aren't quite testing what the description is asking for. If you have multiple tests with exactly the same assertion, there should be a clue that we're looking for something else.
Created instances (by calling new) in Account.all Yes
Used CSV library only in Account.all (not in Account.find) No - you are unnecessarily duplicating the CSV reading code in the find method. You should use the all method in the find method so you are not duplicating this code.
Used Account.all to get account list in Account.find No - see above.
Wave 3
All stubbed tests are implemented fully and pass Yes. There is some opportunity for improvement in DRYing up the tests. You can add a before block and put the .new calls in here.
Used inheritance in the initialize for both Checking and Savings Accounts (min balance) No - no inheritance was used in the initialize methods.
Used inheritance for the withdraw in checking and savings accounts No - no inheritance was used in the withdraw methods.
Baseline
Used Git Regularly You did a good job at the start of the project, but Friday only have one commit. Use some sort of reminder to make sure you are always committing regularly.
Answer comprehension questions Yes. Still seems like there is some confusion with these major topics. We define a class method when the method related to the class as a whole rather than an individual instance of the class. We use inheritance when we want to share some behavior across our classes, when we want to extend the behavior of a class we've created. Account is our parent class and SavingsAccount is an example of a subclass.
Code formatting You need some work still on formatting. You should always use the atom shortcut to fix the formatting. You also need to put one line of whitespace (hitting the "enter" key) between your method definitions (lines 40-41, 46-47 of the Account class)
Overall It seems like you missed the important aspects of inheritance in this assignment. By re-writing each method that was in the Account class, you are not saving yourself any additional work. It does seem like you got good practice with creating tests here, so hopefully you feel more confident with that topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@MonalisaC @kariabancroft